-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[1.20.6] Switch to Forge's fork of Fabric Mixin #9958
base: 1.20.x
Are you sure you want to change the base?
Conversation
I think that the final decision of whether this PR should be merged or not should be up to Lex. I can't speak on his personal feelings and experiences on this, but I wouldn't blame him if his negative experiences working with external libraries causes him to veto merging this PR. It's not a comment Fabric Mixin itself, but it is at least important to keep this on record considering the state of affairs of the general Minecraft modding community. |
Fair enough. One thing I'd like to reiterate is that concerns of safety shouldn't be an issue on the basis that we perform extra scrutiny on it and host our own copy. The goal of this PR isn't to promote Fabric, but to help solve a pain-point for some mod devs and make it easier for them to write safer, more compatible coremods when their use-case is too niche or complex for a PR, which in turn could help the wider Forge modding ecosystem. |
About this PR |
I don't agree. While I use MixinExtras on my own projects, I don't want to get to a point where we just start bloating Forge more than it already has been. |
If MixinExtras functions correctly when jarJar'd then there is no need for us to ship any particular flavor of it. Ideally the official Mixin would release an update making the bulk of Fabric's fork obsolete. But that hasn't happened hence looking for another solution. At the end of the day Mixin is a library Forge doesn't use, and should be treated like any other library that Forge doesn't use. Its not a promotion of condemnation of any variant of Mixin {tho yes I obviously have my own personal misgivings about the fabric one} It's just that Forge itself shouldn't dictate one or the other. However the current state of JarJar is horrible and reportedly doesn't work for Mixin itself. So having a audited self-hosted variant is the best way to go as a interm step. I've spoken with Paint about my stance and left it in his hands. |
Android apps development works completely standalone on apps. Android doesnt provide anything else than a JVM and the android API to do whatever you want This was done by security reasons (and don't bind apps to have a specific set of libraries each android version). And that specific ecosystem works because apps doesn't interact each other and whatever one app do doesn't affect others in runtime (also helps with the retro compatibility) Modding on Minecraft is basically a connected ecosystem between Minecraft, MinecraftForge and the Mods jar. The distribution of these is not same as an Android app because it uses the libraries used on Minecraft and Forge so it can't run standalone. That means it requires a specific set of libraries. Mixins is not just a arbitrary library who doesn't have repercussions on modding, is a commonly used library. And if is a basic tool not used by Minecraft or Forge but is used by the modders it was not bloat. Because is a modding tool like Forge, and if forge can't supply a specific need from a modder then have the mixin/MixinExtra library as a secondary option, so modders doesn't have to jarJar or shade the same library used by other mods. (JarJar is shit for these basic needs) This is just to keep compatibility with other mods, in a future will end in version conflicts (an outdated mod will load earlier an outdated version of mixins) or conflicts with vanilla mixins and fabric mixins (because modders have their own preferences) or forked versions of mixins (because why not?). So that means compatibility is primordial over "let modders do shit". Not because is not used by forge doesn't mean it should be removed just to make modders have to install a mixin mod dependency like the hell was done in 1.12.2 with 6 FUCKING MIXIN BOOTERS DEPENDENCY (and for some reason are appearing more of these) Bloating is basically add libraries not even asked, used or needed on modding, such as put kotlin or Vulkan because that's for a specific mod and not a common use library So please, reconsider that, is basically the worst idea ever had. Specially considering how mature was the state of modding with mixins. |
Lex doesn't intend on unbundling mixin before JiJ is changed to support it properly, and doing so isn't a small task, so I wouldn't worry about it too much at the moment. I'm of the opinion that Forge should bundle mixin & mixinextras for the same reason we offer popular events for mods to use that forge itself doesn't use. It's not much work to bundle and encourages their use over less friendly solutions like js coremods. However, if it becomes a single line of code in a buildscript to JiJ mixin in the future and it resolves to the latest version of whatever mods pick (no bootstrap mods required), then while not ideal, I don't think it's the end of the world. It means people aren't reliant on Forge to update the bundled versions, and isn't complicated for mods to add. |
Ya, I can't really type this without sounding like an ass. But your wall of text is both irrelevant to the current discussion. And completely misunderstanding/failing to read what has been said. It will not cause compatibility issues in any more sense then the core issues using Mixins inherently causes. |
Mixins are essential for modding nowadays. It allows authors to go beyond the limits of a mod loader. If something doesn't exist for which there is a need, it can be created. Without having to redesign or PR changes for just one feature, that someone else wouldn't need. Using the Multiloader template myself, it's often easier to use mixins to solve a problem than using a hacky solution for each. Even if Forge in itself doesn't use it, it seems like a major oversight removing its built-in support. While I appreciate the Forge teams continued development in keeping everything updated, the trend seems to be towards temporary solutions currently. Keeping mixin compatibility without having the need to JiJ or depend on external dependencies which bloats mods needlessly would be much preferred. Just wanted to voice my opinion on this. To remain on topic, using the Fabric mixin and mixin extras seems like a very good move to me. Especially for being able to use the same mixin code in a multiloader environment. Unfortunately users have to choose between 3 mod loaders, I'm trying to keep support for all of them. It would be great to have that same mindset for Forge, regardless of the bad history that has happened. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Locking the conversation as nobody is providing any useful contributions. Nore are they taking the time to understand anything that is said by me. My entire point is that I do not want to deal with that type of stupidity/drama. I am not an incompetent programmer, nore am I ignorant about Mixin's place in the community. There is literally no downside. The course of actions has been decided, and will move forward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good enough for a stop-gap until JiJ is re-designed with needed functionality.
This PR switches Forge from Sponge's Mixin to a fork of Fabric's, which offers various benefits, such as:
Support for
"compatibilityLevel": "JAVA_21"
in mixin json filesJAVA_13
, same as Forge does for"compatibilityLevel": "JAVA_17"
Support for interface mixins (FabricMC Mixin PR 51)
Improved performance/efficiency/memory usage due to avoiding allocations of unused
CallbackInfo
objects (FabricMC Mixin PR 52)Known limitations:
Quick Q&A
A: We've given him ample time to do this - much longer than downstream did, but we understand life can get busy and don't want to rush him. To avoid stagnation and help mod devs, this PR was made as a stop-gap solution. Once upstream Mixin catches up, we can switch back.
A: We'll host our own fork that of Fabric Mixin that we'll keep up to date, so any concerns about that (however valid or paranoid they may be) are avoided.
A: You can either use this installer made for your convenience or do the usual testing process of cloning the PR branch, running the
setup
gradle task andpublish
: forge-1.20.6-50.0.7-1.20.6-fabric-mixin-installer.zipA: Not to my knowledge - it should be a drop-in replacement. Did some testing and didn't run into any problems myself. The only change for Forge mod devs writing mixins from 1.20.4 is about no longer needing the MixinGradle plugin or refmaps, as 1.20.6 uses MojMap/official runtime mappings.